Make the shell scripts runnable, and have them find a Python 3 - #137
Merged
dmccoystephenson merged 3 commits intoAug 22, 2026
Merged
Conversation
test.sh, run.sh and format.sh each documented themselves as ./<name>.sh while being tracked non-executable with a commented-out shebang, so a fresh clone got Permission denied from the one command a contributor is told to run before opening a PR. All three now carry #!/bin/bash and the executable bit. test.sh and run.sh also hardcoded `python`, which is 2.7 on some systems and absent on others; under 2.7 the failure surfaced as a conftest ImportError for pygame rather than as a wrong interpreter. Both now pick the first of python3/python that reports 3.8 or newer, honour a PYTHON override for a virtualenv interpreter, and say so plainly when neither candidate qualifies. README and requirements.txt spell the run command python3, and README documents the override. The coverage artifacts test.sh writes are gitignored so running the gate no longer dirties the tree. Closes #135 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first pass at these tests asserted that particular strings appeared in test.sh and run.sh, which would have passed on a resolver that was worded right and behaved wrong. The resolver's function body is now lifted out of the script under test and run against fake python3/python executables that report whatever version a case needs, so preference order, the fallback, the PYTHON override, the version floor and the failure exit are each observed rather than inferred. The floor the tests hold the resolver to is parsed out of README, so the scripts and the requirement they implement cannot drift apart quietly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Without bash on PATH the resolver cases handed subprocess.run a None executable and died on a TypeError, which says nothing about the scripts. They are marked to skip instead; the shebang and executable-bit checks need no subprocess and still run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
Self-reviewScored against the local suite as the external anchor (
Findings
Two rubric items are scored FAIL above, both judgment calls rather than defects, both left in place with the reasoning stated so a reviewer can decide against them. This review was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener). drafted by Claude on behalf of Daniel Stephenson |
dmccoystephenson
deleted the
feature/shell-script-interpreter-resolution
branch
August 22, 2026 02:29
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
test.sh,run.shandformat.share given a real#!/bin/bashshebang in place of the commented-out# /bin/bash, and the executable bit is set on all three, so the./<name>.shinvocation each one documents in its own usage line works on a fresh clone instead of failing withPermission denied.test.shandrun.shno longer hardcodepython. An interpreter is resolved first: the first ofpython3, thenpython, that reports 3.8 or newer — the minimum README states — is taken, and aPYTHONenvironment variable set beforehand overrides the choice so a virtualenv interpreter can be pointed at directly. When no candidate qualifies, a plain message is printed and the script exits 1, rather than the run proceeding under Python 2.7 and surfacing as aconftestImportErrorfor pygame.README.mdand in the comment atrequirements.txt:2are spelledpython3, and the interpreter-resolution behaviour and thePYTHONoverride are documented in README's usage section.test.shwrites (.coverage,cov.xml) are added to.gitignore. This is adjacent rather than named by the issue, and is included deliberately: with./test.shnow runnable on a fresh clone, the project's only verification gate would otherwise leave two untracked files in the tree every time it is run — the same class of stray-artifact leak that commitad85816had to undo.tests/test_shell_scripts.pyis added as a regression guard, since none of the above is exercised by the existing suite. The resolver is run rather than pattern-matched: its function body is lifted out of the script under test and executed against fakepython3/pythonexecutables that report whatever version a case needs, withPATHholding nothing else. Preference order, the fallback topython, thePYTHONoverride, the version floor and the failure exit are each observed. The floor is parsed out of README, so the scripts and the requirement they implement cannot drift apart quietly. The shebang, the executable bit, and the fact that every line running pytest or the game goes through"$PYTHON"are asserted statically alongside.Test plan
python3 -m pytest— 306 passed, 0 failed (289 before this change, plus 17 new script tests)../test.shitself could not be invoked in the session this change was prepared in — the sandbox refuses to execute repository shell scripts — so the commandtest.shissues was run directly instead.src/is touched by this change, and total coverage is unchanged at 87%.test.sh,run.sh,format.sh,README.mdandrequirements.txtrestored to theirorigin/mainstate and the new test file left in place, 16 of the 17 new tests FAIL (the seventeenth is abash -nsyntax check, which the old scripts also pass); with the change restored, all 17 PASS.(3, 0)failstest_the_documented_minimum_version_is_the_one_enforced[test.sh]and nothing else; reversing the candidate order topython python3failstest_the_newest_qualifying_candidate_is_preferred[test.sh]and nothing else. Both mutations were then reverted and the suite reconfirmed green.__pycache__,.pyc,save.json,output.txt,.coverage,cov.xmlor.claude/path is included, and the three scripts are recorded in the index at mode100755../test.shand./run.shon an ordinary machine is still worth doing before merge, since the end-to-end invocation is the one thing the sandbox could not cover and this repository has no CI workflow to cover it either.Notes
src/is modified, so no player-facing behaviour changes and the UI-parity question between the pygame and text interfaces does not arise here.checkDependenciesblock inrun.shstill mentions barepython; it is dead code and was left untouched to keep the diff to the issue's scope.tests/test_shell_scripts.pydoes not mirror a module undersrc/the way every other test file does, because the code it covers is not Python. It is placed at the top level oftests/alongside the other non-package test files.Deferred this cycle
The rest of the open backlog was not picked up, for these reasons:
src/, and are plausibly coupled to each other — batching either with a shell-script and documentation fix would have mixed unrelated concerns in one review. They are good candidates for the next cycle, taken together.Four pull requests were already open at triage — #59 (
develop, conflicting), and Copilot drafts #98, #96 and #90 (two conflicting) — all between nine and fourteen months stale and authored elsewhere. None was opened by this loop, so none was adopted or closed here; a decision on them belongs to a human.Closes #135
This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).
drafted by Claude on behalf of Daniel Stephenson